Skip to content

fix: map skill references for skills-only delivery#891

Open
Tylor110077 wants to merge 1 commit intoFission-AI:mainfrom
Tylor110077:codex/fix-skills-only-command-refs
Open

fix: map skill references for skills-only delivery#891
Tylor110077 wants to merge 1 commit intoFission-AI:mainfrom
Tylor110077:codex/fix-skills-only-command-refs

Conversation

@Tylor110077
Copy link
Copy Markdown

@Tylor110077 Tylor110077 commented Mar 30, 2026

Summary

  • add transformToSkillReferences() for skills-only delivery
  • wire the transformer in init and update
  • add tests for command-reference transformation and skills-only init output

Testing

  • npx vitest run test/utils/command-references.test.ts test/core/init.test.ts
  • npm run build

Closes #879

Summary by CodeRabbit

  • New Features
    • Enhanced SKILL.md generation to support skill references in skills delivery mode, mapping workflow identifiers to their corresponding skill references.

@Tylor110077 Tylor110077 requested a review from TabishB as a code owner March 30, 2026 15:59
Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 112fd556-6f18-4e7f-b09b-d1475ffef060

📥 Commits

Reviewing files that changed from the base of the PR and between afdca0d and 5fb16bd.

📒 Files selected for processing (5)
  • src/core/init.ts
  • src/core/update.ts
  • src/utils/command-references.ts
  • test/core/init.test.ts
  • test/utils/command-references.test.ts

📝 Walkthrough

Walkthrough

The PR fixes a bug where skills-only delivery mode generated SKILL.md files containing /opsx:* command references instead of /openspec-* skill namespace equivalents. It introduces a new transformer function that maps these hardcoded references to their corresponding skill cross-references and wires it into the skill generation flow.

Changes

Cohort / File(s) Summary
Transformer Logic
src/core/init.ts, src/core/update.ts
Updated SKILL.md generation to conditionally apply transformToSkillReferences for non-opencode tools when delivery === 'skills', replacing the hardcoded /opsx:* references with /openspec-* equivalents in generated skill files.
Command Reference Utilities
src/utils/command-references.ts
Added OPSX_TO_SKILL_REFERENCE mapping object (8 workflow mappings) and new exported function transformToSkillReferences() that scans and replaces /opsx:<workflow> patterns with corresponding /openspec-* skill references.
Test Suite Updates
test/core/init.test.ts, test/utils/command-references.test.ts
Updated delivery=skills test to expect /openspec-* references; added comprehensive test suite for transformToSkillReferences covering known/unknown workflow mappings and edge cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • TabishB

Poem

🐰 Cross-references once broken, now refine,
From /opsx: commands to skill designs divine,
Skills-only mode now gleams so bright,
With /openspec-* paths set right! 🌟

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: mapping skill references for skills-only delivery mode, which directly addresses the bug fix.
Linked Issues check ✅ Passed The PR fully implements the suggested fix from issue #879: adds transformToSkillReferences() function with explicit mapping, wires it into init/update paths when delivery==='skills', and validates with comprehensive tests.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing issue #879: new transformer function, conditional integration in generation paths, and tests validating the fix. No unrelated changes detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@TabishB
Copy link
Copy Markdown
Contributor

TabishB commented Apr 10, 2026

Hey @Tylor110077, thanks for digging into this! The bug is real and your fix correctly addresses the symptoms.

After reviewing, I want to flag something before we merge. The core issue here is that transformer selection logic has to be duplicated across init.ts and update.ts (3 places total), and the precedence between tool type and delivery mode is getting tricky to reason about (e.g., what should happen when the tool is OpenCode and delivery is skills-only?).

We have an existing change proposal called "unify-template-generation-pipeline" that was designed to solve exactly this class of problem. It introduces a first-class transform pipeline where each transform declares when it applies, so instead of duplicated ternaries we'd have a single declarative transform with an explicit applies(ctx) function. That would make the OpenCode vs. skills-only precedence question impossible to get wrong.

We're going to prioritize that change, and this fix would land naturally as part of it.

I don't want your work to go to waste though. Your mapping table (OPSX_TO_SKILL_REFERENCE) and the test cases are solid and will carry over directly into the unified pipeline. We'll reference this PR when we implement that piece.

Holding off on merging for now, but really appreciate you tracking this down and writing it up cleanly. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Skills-only delivery emits /opsx:* command references

2 participants